Add password support to Redis chat memory autoconfigure#6637
Conversation
Expose spring.ai.chat.memory.repository.redis.password and wire it into the Jedis RedisClient when configured. Keep legacy spring.ai.chat.memory.redis prefix delegation and document the new property. Fixes spring-projects#6621 Signed-off-by: Bishen Yu <aukovyps@163.com>
|
Hey @dimitarproynov — would you mind taking a look when you get a chance? This adds optional password support to Redis chat memory autoconfigure (spring.ai.chat.memory.repository.redis.password), with the legacy spring.ai.chat.memory.redis.* prefix still delegated. |
| public RedisClient jedisClient(RedisChatMemoryRepositoryProperties properties) { | ||
| if (StringUtils.hasText(properties.getPassword())) { | ||
| JedisClientConfig clientConfig = DefaultJedisClientConfig.builder() | ||
| .password(properties.getPassword()) |
There was a problem hiding this comment.
I wonder why not add "user" as well. This is a nit and doesn't block the merge.
dimitarproynov
left a comment
dimitarproynov
left a comment
There was a problem hiding this comment.
Thank you for your contribution!
The change is tidy, but we need to work on the unit tests.
| @Test | ||
| void passwordWithOldPrefix() { | ||
| this.contextRunner | ||
| .withPropertyValues("spring.ai.chat.memory.redis.host=10.0.0.1", "spring.ai.chat.memory.redis.port=6380", | ||
| "spring.ai.chat.memory.redis.password=secret", | ||
| "spring.ai.chat.memory.redis.initialize-schema=false") | ||
| .run(context -> { | ||
| var chatProperties = context.getBean(RedisChatMemoryRepositoryProperties.class); | ||
| assertThat(chatProperties.getPassword()).isEqualTo("secret"); | ||
| }); | ||
| } | ||
|
|
||
| @Test | ||
| void passwordWithNewPrefix() { | ||
| this.contextRunner | ||
| .withPropertyValues("spring.ai.chat.memory.repository.redis.host=10.0.0.1", | ||
| "spring.ai.chat.memory.repository.redis.port=6380", | ||
| "spring.ai.chat.memory.repository.redis.password=secret", | ||
| "spring.ai.chat.memory.repository.redis.initialize-schema=false") | ||
| .run(context -> { | ||
| var chatProperties = context.getBean(RedisChatMemoryRepositoryProperties.class); | ||
| assertThat(chatProperties.getPassword()).isEqualTo("secret"); | ||
| }); |
There was a problem hiding this comment.
Both of these test cases test the property parser, rather than the fact that the RedisClient in the autoconfiguration was built with JedisClientConfig with password set. Maybe extensive reflection must be used to assert that the RedisClient has a password in its ConnectionProvider, but otherwise we are not testing the change.
3 participants
Fixes #6621
Summary
Redis chat memory autoconfiguration supports
hostandport, but notpassword. Users with password-protected Redis instances had to provide their ownRedisClientbean instead of usingspring.ai.chat.memory.repository.redis.*properties.This PR adds an optional
passwordproperty and wires it into the JedisRedisClientwhen configured. The legacyspring.ai.chat.memory.redis.*prefix continues to work via delegation.Changes
passwordtoRedisChatMemoryRepositoryPropertiesRedisClientinRedisChatMemoryRepositoryAutoConfigurationpasswordin deprecatedRedisChatMemoryPropertiesfor the legacy prefixspring.ai.chat.memory.repository.redis.passwordinchat-memory.adocConfiguration
Legacy prefix (still supported):
Test plan